Skip to content

fix: post-PR#2 regressions + regenerate README#3

Merged
stuforfun merged 6 commits intomainfrom
dev-claude
Mar 8, 2026
Merged

fix: post-PR#2 regressions + regenerate README#3
stuforfun merged 6 commits intomainfrom
dev-claude

Conversation

@stuforfun
Copy link
Owner

Follow-up to PR #2 (d1ac616). Two regressions caught in post-merge review:

Fixes

  • Viewport meta restored — PR fix: destination card color coherence + UX polish #2 accidentally replaced width=device-width, initial-scale=1.0 with notranslate tags; mobile browsers were rendering at ~980px desktop width
  • @Keyframes flash-dest added — destination card outboxes referenced this animation but it was never defined, silently breaking flash-on-update feedback

Docs

  • README regenerated with verified city counts (1,055 entries, 1,039 unique names, 44 aliases) and accurate technical details

Reviewed by: @codex review

stuforfun and others added 2 commits March 7, 2026 14:35
- Restore <meta name="viewport" content="width=device-width, initial-scale=1.0">
  accidentally removed in d1ac616 (PR #2); mobile browsers were falling
  back to ~980px desktop width and scaling down the UI
- Add @Keyframes flash-dest definition missing from d1ac616; destination
  card outboxes were referencing the animation but it was never defined,
  silently breaking flash-on-update feedback

Co-authored-by: stuforfun <stuforfun@users.noreply.github.com>
- Verified city count: 1,055 entries, 1,039 unique names, 44 aliases
- Documents DST-awareness, fuzzy search, half/quarter-hour offsets
- Adds technical reference table and local dev instructions
@chatgpt-codex-connector
Copy link

To use Codex here, create an environment for this repo.

@stuforfun
Copy link
Owner Author

Codex review: I re-reviewed the two commits currently unique to this branch and found no new diff-specific issues.

  • 6382d1f correctly restores the viewport meta tag and adds the missing flash-dest keyframes.
  • 0400ada is documentation-only, and the updated counts check out against index.html: 1,055 entries, 1,039 unique city names, 44 aliases.

Residual repo issues worth tracking, but not introduced by these two commits:

  1. README.md still claims persistent state, but index.html clears localStorage on startup, so persistence is effectively disabled.
  2. README.md still says "zero CDN at runtime," but index.html still loads Google Fonts at runtime.

- Remove false persistence claim: localStorage is cleared on startup
  via removeItem() in the init IIFE; app is stateless between visits
- Fix CDN claim: Google Fonts is loaded from CDN at runtime;
  corrected 'zero CDN' to reflect actual behaviour
Init IIFE was calling removeItem(STORAGE) unconditionally,
wiping any saved state before it could be loaded. loadState()
and saveState() were fully implemented but persistence was
effectively disabled.

Now: attempt loadState() on startup; if a saved session exists,
restore origin city, timezone, format, date, time, and all
destination cards, then convert. Fall back to fresh start if
no saved state found.
The stateless note was added after Codex caught removeItem() wiping
state on startup. That bug is now fixed in af45417; README updated
to accurately reflect working persistence again.
@stuforfun
Copy link
Owner Author

Codex review: I found one additional branch-head issue in the startup restore path.

  1. In af45417, the app restores fromTz during init and then calls convertAll(), but the origin timezone box is only populated in the origin dropdown select handler. convertAll() flashes #origin-tz, but it never recomputes or writes its text. After a reload, a restored session can therefore show correct destination conversions while the origin timezone field still displays the placeholder.

This makes the restored session internally inconsistent even though the conversion math is using the saved timezone.

Codex review (PR #3) caught that af45417 restored fromTz correctly
but never updated the #origin-tz display element. After reload, the
conversion math was correct but the origin timezone field still showed
the placeholder text, creating an inconsistent UI.

Fix: after restoring fromTz from saved state, set origin-tz.textContent
to utcLabel(s.tz) and add the 'live' class, matching what the dropdown
select handler does on a fresh city pick.
@stuforfun
Copy link
Owner Author

Codex review: I re-reviewed the current branch head (1140f99) after the follow-up fix for session restore.

I do not see any new diff-specific issues in the branch at this point. The previous restore-path inconsistency appears addressed: init now repopulates #origin-tz from saved state before the restored conversion runs.

Residual risk: this is still based on code inspection only. I did not run the app in a browser here, so the remaining gap is end-to-end coverage around reload/restore behavior.

@stuforfun stuforfun merged commit f0b0c91 into main Mar 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant